Skip to content

Conversation

KyleAMathews
Copy link
Collaborator

Summary

Implements GitHub issue #402 by adding flexible matching strategies for electric-db-collection synchronization.

Txid Strategy (existing, backward compatible) - Uses PostgreSQL transaction IDs for precise matching
Custom Match Function Strategy (new) - Allows heuristic-based matching with custom logic
Void/Timeout Strategy (new, 3-second default) - Simple timeout for prototyping

Key Features

  • New types: MatchFunction<T>, MatchingStrategy<T>
  • Enhanced ElectricCollectionConfig to support all strategies
  • New utility: awaitMatch(matchFn, timeout?)
  • Export isChangeMessage and isControlMessage helpers for custom match functions
  • Comprehensive test coverage for all strategies
  • Updated documentation with detailed examples and migration guide

Benefits

  • Backward Compatibility: Existing code works unchanged
  • Architecture Flexibility: Choose the best strategy for your backend capabilities
  • Progressive Enhancement: Start with void strategy, upgrade to txid when ready
  • No Forced Backend Changes: Custom match functions work without backend modifications

Test plan

  • All existing tests pass
  • New tests for all three matching strategies
  • Timeout behavior testing
  • Backward compatibility verification
  • Integration tests with Electric stream simulation
  • Linting and type checking passes

🤖 Generated with Claude Code

- Add three matching strategies for client-server synchronization:
  1. Txid strategy (existing, backward compatible)
  2. Custom match function strategy (new)
  3. Void/timeout strategy (new, 3-second default)

- New types: MatchFunction<T>, MatchingStrategy<T>
- Enhanced ElectricCollectionConfig to support all strategies
- New utility: awaitMatch(matchFn, timeout?)
- Export isChangeMessage and isControlMessage helpers
- Remove deprecated error classes (beta compatibility not required)
- Comprehensive tests for all strategies including timeout behavior
- Updated documentation with detailed examples and migration guide

Benefits:
- Backward compatibility maintained
- Architecture flexibility for different backend capabilities
- Progressive enhancement path
- No forced backend API changes

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link

changeset-bot bot commented Sep 3, 2025

🦋 Changeset detected

Latest commit: 89e7cce

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@tanstack/electric-db-collection Minor
@tanstack/db-example-react-todo Patch
@tanstack/db-example-solid-todo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Sep 3, 2025

More templates

@tanstack/angular-db

npm i https://pkg.pr.new/@tanstack/angular-db@499

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@499

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@499

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@499

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@499

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@499

@tanstack/rxdb-db-collection

npm i https://pkg.pr.new/@tanstack/rxdb-db-collection@499

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@499

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@499

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@499

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@499

commit: 89e7cce

Copy link
Contributor

github-actions bot commented Sep 3, 2025

Size Change: +14 B (+0.02%)

Total Size: 75.2 kB

Filename Size Change
./packages/db/dist/esm/collection/mutations.js 2.52 kB +14 B (+0.56%)
ℹ️ View Unchanged
Filename Size
./packages/db/dist/esm/collection/change-events.js 943 B
./packages/db/dist/esm/collection/changes.js 1.01 kB
./packages/db/dist/esm/collection/events.js 660 B
./packages/db/dist/esm/collection/index.js 3.18 kB
./packages/db/dist/esm/collection/indexes.js 1.16 kB
./packages/db/dist/esm/collection/lifecycle.js 1.8 kB
./packages/db/dist/esm/collection/state.js 3.82 kB
./packages/db/dist/esm/collection/subscription.js 1.65 kB
./packages/db/dist/esm/collection/sync.js 1.42 kB
./packages/db/dist/esm/deferred.js 230 B
./packages/db/dist/esm/errors.js 3.1 kB
./packages/db/dist/esm/index.js 1.56 kB
./packages/db/dist/esm/indexes/auto-index.js 806 B
./packages/db/dist/esm/indexes/base-index.js 835 B
./packages/db/dist/esm/indexes/btree-index.js 2 kB
./packages/db/dist/esm/indexes/lazy-index.js 1.21 kB
./packages/db/dist/esm/indexes/reverse-index.js 577 B
./packages/db/dist/esm/local-only.js 827 B
./packages/db/dist/esm/local-storage.js 2.02 kB
./packages/db/dist/esm/optimistic-action.js 294 B
./packages/db/dist/esm/proxy.js 3.86 kB
./packages/db/dist/esm/query/builder/functions.js 615 B
./packages/db/dist/esm/query/builder/index.js 4.04 kB
./packages/db/dist/esm/query/builder/ref-proxy.js 938 B
./packages/db/dist/esm/query/compiler/evaluators.js 1.55 kB
./packages/db/dist/esm/query/compiler/expressions.js 631 B
./packages/db/dist/esm/query/compiler/group-by.js 2.04 kB
./packages/db/dist/esm/query/compiler/index.js 2.04 kB
./packages/db/dist/esm/query/compiler/joins.js 2.52 kB
./packages/db/dist/esm/query/compiler/order-by.js 1.21 kB
./packages/db/dist/esm/query/compiler/select.js 1.28 kB
./packages/db/dist/esm/query/ir.js 785 B
./packages/db/dist/esm/query/live-query-collection.js 340 B
./packages/db/dist/esm/query/live/collection-config-builder.js 2.69 kB
./packages/db/dist/esm/query/live/collection-subscriber.js 1.86 kB
./packages/db/dist/esm/query/optimizer.js 3.08 kB
./packages/db/dist/esm/SortedMap.js 1.24 kB
./packages/db/dist/esm/transactions.js 3 kB
./packages/db/dist/esm/utils.js 1.01 kB
./packages/db/dist/esm/utils/browser-polyfills.js 365 B
./packages/db/dist/esm/utils/btree.js 6.01 kB
./packages/db/dist/esm/utils/comparison.js 754 B
./packages/db/dist/esm/utils/index-optimization.js 1.73 kB

compressed-size-action::db-package-size

Copy link
Contributor

github-actions bot commented Sep 3, 2025

Size Change: 0 B

Total Size: 1.47 kB

ℹ️ View Unchanged
Filename Size
./packages/react-db/dist/esm/index.js 152 B
./packages/react-db/dist/esm/useLiveQuery.js 1.32 kB

compressed-size-action::react-db-package-size

KyleAMathews and others added 4 commits September 3, 2025 15:35
…nd API consistency

Critical fixes based on thorough code review:

**🔧 Commit Semantics Fix:**
- awaitMatch now waits for up-to-date after finding match (like awaitTxId)
- Ensures consistent behavior between txid and custom match strategies
- Prevents race conditions where mutations marked "persisted" before actual commit

**🧠 Memory Leak Fixes:**
- Properly cleanup pendingMatches on timeout and abort
- Add abort listener to cleanup all pending matches on stream abort
- Use cross-platform ReturnType<typeof setTimeout> instead of NodeJS.Timeout

**🎯 API Consistency:**
- Unified MatchingStrategy type used across all handler return types
- Support configurable timeout for void strategy: { timeout: 1500 }
- Remove unused discriminator type field for cleaner duck-typed unions

**🧪 Enhanced Test Coverage:**
- Test memory cleanup after timeout (no lingering handlers)
- Test commit semantics (awaitMatch waits for up-to-date)
- Test configurable void timeout functionality
- All edge cases now properly covered

**📦 Version Bump:**
- Changeset updated to minor (removed exported error classes)

All feedback addressed while maintaining backward compatibility.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Based on engineering feedback, this commit addresses several critical edge cases:

**Memory Safety & Error Handling:**
- Fix timeout cleanup memory leak in awaitMatch - pending matchers now properly removed on timeout
- Add try/catch around matchFn calls to prevent user code from crashing stream loop
- Add proper abort semantics with StreamAbortedError for pending matches
- Add TimeoutWaitingForMatchError following codebase error class conventions

**Race Condition Fix:**
- Implement up-to-date bounded message buffer to handle race where messages arrive before matcher registration
- Buffer is safely bounded to current transaction batch, eliminating stale data matching risks
- Messages cleared on each up-to-date to maintain transaction boundaries

**Test Reliability:**
- Replace timing-based assertions with fake timers using vi.runOnlyPendingTimersAsync()
- Eliminates CI flakiness while testing the same void strategy functionality

**Cross-platform Compatibility:**
- Confirmed ReturnType<typeof setTimeout> usage for browser compatibility
- API shape consistency already matches runtime behavior

The core matching strategy design (txid/custom/void) remains unchanged - these are
lifecycle polish fixes for production readiness.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
KyleAMathews and others added 3 commits October 6, 2025 16:44
Resolved merge conflicts in electric.ts by combining:
- Custom match function support from match-stream branch
- Snapshot support from origin/main

Both features are now integrated together.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
The interface was extending BaseCollectionConfig with a strict handler
return type of { txid: ... }, but our new matching strategies support
broader return types including matchFn and void strategies.

Removed the extends constraint and manually included needed properties
to allow handlers to return any MatchingStrategy type.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
*/
id?: string
schema?: TSchema
getKey: CollectionConfig<T, string | number, TSchema>[`getKey`]
Copy link
Contributor

@kevin-dp kevin-dp Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these properties and methods (getKey, onInsert, etc.) are defined in the BaseCollectionConfig. Let's not introduce them here but extend the BaseCollectionConfig class as it used to do and only add Electric-specific things that are not part of the base config.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR originally predates the refactor when BaseCollectionConfig was introduced - all these props were here and modified in the first commit to this PR. I think Claude has not caught that context when addressing the merge conflict when merging in main.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i was expecting that it had rebased correctly. Anyways, it will have to do that :)

@kevin-dp kevin-dp self-requested a review October 7, 2025 11:38
directOpTransaction.commit().catch(() => undefined)

// Add the transaction to the collection's transactions store
state.transactions.set(directOpTransaction.id, directOpTransaction)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the commit fails, do we still want to store this TX ID in the state.transactions?

Copy link
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the comment on removing the duplicate interface we needs addressing before being merged, all look ok.

Question I do have is on why have two different methods for awaiting a match. We have this returned object with a txid or match function, as well as the utility functions to do the same. Could we standardise on just one way to do this?

The advantages of the unity methods is that you can compose multiple together, awaiting both/either a txid or another match, where with the returned value you can't. If a mutation functions calls multiple backend apis that all do inserts/udates, there could be multiple txids to await, the utility functions handle that well.

I think it also makes it conceptional easer to understand, your mutation function returns when the transaction is over, rather than it returning a value that tells the system when the transaction is over.

I'm approving as I know you are keen to get this out, but wanted to flag those thoughts.

Each handler can return:
- `{ txid: number | number[] }` - Txid strategy (recommended)
- `{ matchFn: (message) => boolean, timeout?: number }` - Custom match function strategy
- `{}` - Void strategy (3-second timeout)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we let a user return the timeout on the void strategy?

Suggested change
- `{}` - Void strategy (3-second timeout)
- `{ timeout?: number }` - Void strategy (default 3-second timeout)

await awaitTxId(txid)
// Check against current batch messages first to handle race conditions
for (const message of currentBatchMessages.state) {
if (checkMatch(message)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is checkMatch looking up the match and setting match.matched to true if we are also doing that here on L465? Isn't L465 going to override whatever we set on L449?

if (`txid` in result) {
// Handle both single txid and array of txids
if (Array.isArray(result.txid)) {
await Promise.all(result.txid.map((id) => awaitTxId(id)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can shorten result.txid.map((id) => awaitTxId(id)) to result.txid.map(awaitTxId)

Copy link
Contributor

@kevin-dp kevin-dp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, this is almost ready to be merged. We just need to fix the rebase that went a bit wrong (i.e. use the BaseCollectionConfig to inherit common properties) and extract some of the logic to helper functions.


// Check pending matches against this message
// Note: matchFn will mark matches internally, we don't resolve here
const matchesToRemove: Array<string> = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth extracting this logic to clean up matches into a helper function and call the helper function here in order to keep the code readable and concise.

})

// Resolve all matched pending matches on up-to-date
const matchesToResolve: Array<string> = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth also extracting this logic to clean up matches into a helper function and call the helper function here in order to keep the code readable and concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants